fix(web): capitalize project name display across all five sites#2301
fix(web): capitalize project name display across all five sites#2301neogenix wants to merge 13 commits into
Conversation
lefarcen
added
size/XL
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b844d61849
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /* Same display-layer capitalize as .app-project-title .title — see that rule for the | ||
| brand-name trade-off note. Applies to project names and live-artifact titles. */ | ||
| text-transform: capitalize; |
There was a problem hiding this comment.
This rule changes .design-card-name globally, but that class is used for both project names and live-artifact titles in DesignsTab (the live-artifact card renders artifact.title with the same class). That means a fix intended for project-name display also rewrites unrelated artifact titles, so mixed-case or intentionally styled artifact names are shown incorrectly. Please scope this transform to project-name rendering only (or split the class) so artifact titles keep user-provided casing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I'm not sure this is a bad thing. I'm open to changing it, but we might want to either split out the class, or apply the fix universally.
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
Thanks for pushing this through. I found two places where the new shared selectors still rewrite non-project text, so I left inline notes on the spots that need narrower scoping.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.| /* Same display-layer capitalize as .app-project-title .title — see that rule for the | ||
| brand-name trade-off note. Applies to all tab types (project, home, marketplace). */ | ||
| text-transform: capitalize; |
There was a problem hiding this comment.
These shared tab-label selectors also style non-project tabs. WorkspaceTabsBar.displayTabFor() feeds the same elements entry and marketplace titles like entry.navDesignSystems and workspaceTabs.marketplace, so this rule will retitle those localized labels too (Design systems → Design Systems, etc.). That broadens the fix beyond project names and can produce incorrect copy in locales that intentionally do not capitalize every word. Please scope the transform to project tabs only—for example by adding a project-only modifier class when tab.kind === project and moving text-transform onto that class instead of the shared tab-label selectors.
| /* Same display-layer capitalize as .app-project-title .title — see that rule for the | ||
| brand-name trade-off note. Applies to project names and live-artifact titles. */ | ||
| text-transform: capitalize; |
There was a problem hiding this comment.
.design-card-name is shared by both project cards and live-artifact cards in DesignsTab—the live-artifact branch renders artifact.title with this same class. With text-transform here, arbitrary artifact titles now get rewritten too, so values with intentional casing (for example brand names or version strings) display incorrectly even though this PR is meant to touch project names only. Please split the project-name styling from the shared card-title class, or add a project-only class on the project-card path and leave artifact titles on the existing selector without text-transform.
There was a problem hiding this comment.
I'm debating how best to respond to the feedback. The fix should probably be applied to all of them, but the feedback is correct given the naming / usage / etc... I'll noodle on it, and likely focus on the individual fix.
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
Thanks for tightening the selector scoping here. I found one blocker in the new Playwright coverage: the last case still asserts the old shared capitalize behavior on entry tabs.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.
|
Fixed in 399133f: flipped the nav-tab |
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
Thanks for the follow-up here. I found one remaining Designs tab path where project names still skip the new capitalize styling, so I left the concrete spot inline.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.| <div className="design-card-meta-block"> | ||
| <ProjectTag category={projectCategory(p)} /> | ||
| <div className="design-card-name" title={p.name}> | ||
| <div className="design-card-name design-card-name--project" title={p.name}> |
There was a problem hiding this comment.
Nice catch scoping the grid-card selector. One remaining path in this same component still bypasses the new rule: the kanban branch renders project names with .design-kanban-card-name at DesignsTab.tsx:716-720, and apps/web/src/index.css:9063-9069 still has no text-transform for that selector. That means a project called acme studio is fixed in grid view but flips back to lowercase as soon as the user switches the Designs tab to kanban, which is the same consistency bug this PR is trying to remove. Please apply the same project-name casing treatment to the kanban card title path as well, and extend the Vitest/Playwright coverage to exercise the kanban view so this view-mode regression is caught next time.
|
Fixed in 00e08f2: applied |
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
@neogenix I re-checked the selector scoping and the follow-up coverage on this head: project-only modifier classes are now used for the tab labels and grid cards, the kanban card path has its own capitalize rule, and the updated regression tests cover both the project-name paths and the non-project tab guard. Thanks for iterating on the follow-up fixes 🙌
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.
neogenix
force-pushed
the
fix/web-design-files-title-case
branch
2 times, most recently
from
0021b4b to
08902b7
Compare
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
Thanks for iterating on the title-casing follow-up. I found one blocker in WorkspaceTabsBar on this head that needs a quick fix before the branch is merge-ready.
| {hiddenTabCount > 0 ? ( | ||
| <button | ||
| type="button" | ||
| className="workspace-tab workspace-tab--overflow" | ||
| onClick={() => setTabsMenuOpen(true)} | ||
| title="Show hidden tabs" | ||
| > | ||
| {hiddenTabCount} more |
There was a problem hiding this comment.
hiddenTabCount is rendered in this new overflow-chip branch, but this component no longer defines that variable anywhere after the strip switched from visibleTabs slicing to state.tabs.map(...). On the current head that leaves WorkspaceTabsBar.tsx with an unresolved identifier, which is enough to break the web build before any of the casing changes can ship. Please either remove this leftover overflow branch or restore a real hiddenTabCount calculation that matches the current strip behaviour.
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
Thanks for the follow-up here — I rechecked the latest head and found one remaining WorkspaceTabsBar regression in the tab chrome layout, so I left the concrete fix inline.
| className={`workspace-tabs-list__item${active ? ' is-active' : ''}`} | ||
| role="option" | ||
| aria-selected={active} | ||
| <div className="workspace-tabs-actions" ref={menuRef}> |
There was a problem hiding this comment.
workspace-tabs-actions is now nested inside .workspace-tabs-strip, but the strip is the horizontally scrollable container (apps/web/src/index.css:571-592 sets overflow-x: auto and flex: 1 1 0). That means the New tab / Search tabs controls become part of the scrollable tab content instead of staying pinned beside it as they did before this diff. Once a user opens enough tabs to overflow the strip, those controls can scroll off-screen and stop being immediately reachable, which regresses the primary tab-management path in the same area this component is trying to improve. Please move workspace-tabs-actions back out so it is a sibling of workspace-tabs-strip again, and keep only the tab pills inside the scrollable strip.
Add text-transform: capitalize to .app-project-title .title so lowercase-stored project names display with capitalized first letters in the design files page header. Red spec: apps/web/tests/styles/project-title-casing.test.ts Red on origin/main, green on this branch.
Adds a jsdom render test that mounts ProjectView with a lowercase project
name ('acme studio'), injects the .app-project-title CSS rules from
index.css into the document head, and asserts that getComputedStyle on
the data-testid='project-title' element returns text-transform: capitalize.
Red/green verified: removing the text-transform rule produces
AssertionError: expected 'none' to be 'capitalize'
Restoring the rule makes the test pass.
The project-title-casing.test.ts in tests/styles/ was a static CSS parser that asserted the same text-transform: capitalize property already covered by the behavior test in tests/components/. Two tests asserting one property is technical debt; delete the simpler one. Also replace the process.cwd()-relative readFileSync path with path.resolve(process.cwd(), ...) and add an explanatory comment. In the jsdom environment import.meta.url is not a file: URL, so process.cwd() is the correct anchor. The original code worked but was undocumented.
text-transform: capitalize is a display-layer workaround for auto-generated all-lowercase project names. Brand names with mid-word capitalisation (e.g. iPhone -> IPhone) will be displayed incorrectly. A future storage-layer fix that preserves input casing on create/rename should remove this rule.
Add text-transform: capitalize to the four additional CSS selectors that render user project names, matching the rule already applied to the design-files page header (.app-project-title .title): - .workspace-tab__label — tab strip in WorkspaceTabsBar - .workspace-tabs-list__title — tab overflow popover list - .design-card-name — projects grid and live-artifact cards in DesignsTab - .recent-projects__card-name — recent-projects strip on the Home view The rule for .recent-projects__card-name lives in src/styles/home/recent-projects.css (the four index.css selectors are co-located with their existing rules). All four selectors also render non-project-name text in some contexts (navigation labels like 'Home', artifact titles). The capitalize rule is safe for those: already-capitalized strings are unaffected, and the brand-name trade-off documented on .app-project-title .title in index.css applies equally here. Tests: add apps/web/tests/components/css-project-name-casing.test.tsx covering all four new sites with the same stylesheet-injection pattern used for the design-files header test. Each test is verified RED on origin/main (CSS rules absent) and GREEN on this branch. Also extend extractProjectTitleRules in ProjectView.title-casing.test.tsx to a general selector list covering all five sites.
The original five-site sweep missed the kanban view's .design-kanban-card-name selector at DesignsTab.tsx:716-720. Project names rendered correctly in the grid view (via .design-card-name--project) but flipped back to lowercase the moment the user switched to kanban. apps/web/src/index.css gains text-transform: capitalize on .design-kanban-card-name. The selector is project-name-only by usage (kanban renders only project cards; live artifacts go through .design-card-name in the grid view), so no per-card modifier class is needed. e2e/ui/project-name-casing.test.ts gains a sixth scenario that toggles DesignsTab to kanban via [data-testid=designs-view-kanban] and asserts the .design-kanban-card-name computed text-transform resolves to 'capitalize'. Header comment updated from five selectors to six.
neogenix
force-pushed
the
fix/web-design-files-title-case
branch
from
fda8db8 to
bf61d02
Compare
Visual regression reviewHead:
10 changed · 7 unchanged · 0 missing baseline · 0 failed Changed cases
Unchanged cases
Visual diff is advisory only and does not block merging. |
| const labelText = await activeTabLabel.textContent(); | ||
| expect(labelText?.trim()).toBeTruthy(); | ||
| expect(labelText?.trim()[0]).toMatch(/[A-Z]/); | ||
|
|
||
| // The CSS rule is scoped to --project modifier; it must NOT apply to this plain nav entry tab. | ||
| const labelTransform = await activeTabLabel.evaluate( | ||
| (el) => window.getComputedStyle(el).textTransform, | ||
| ); | ||
| expect(labelTransform).not.toBe('capitalize'); | ||
|
|
||
| // Navigate to a second entry view and verify the same invariant holds. | ||
| await page.getByTestId('entry-nav-projects').click(); | ||
| await expect(page).toHaveURL(/\/projects$/); | ||
|
|
||
| const projectsTabLabel = page.locator('.workspace-tab.is-active .workspace-tab__label'); | ||
| await expect(projectsTabLabel).toBeVisible(); | ||
|
|
||
| const projectsLabelText = await projectsTabLabel.textContent(); | ||
| expect(projectsLabelText?.trim()).toBeTruthy(); | ||
| expect(projectsLabelText?.trim()[0]).toMatch(/[A-Z]/); | ||
|
|
||
| const projectsTransform = await projectsTabLabel.evaluate( | ||
| (el) => window.getComputedStyle(el).textTransform, | ||
| ); | ||
| expect(projectsTransform).not.toBe('capitalize'); |
There was a problem hiding this comment.
These two nav-label checks now assert expect(...[0]).toMatch(/[A-Z]/) as well as textTransform !== "capitalize". The behavior this test needs to protect is the selector scoping, but the regex assertions also make the case depend on the runner using an English-like locale. detectInitialLocale() in apps/web/src/i18n/index.tsx:85-101 initializes from navigator.languages / navigator.language, so on a non-English Playwright locale the tab text can be correct and still fail this test because it does not start with ASCII [A-Z]. Please drop the ASCII-capital checks here, or assert against the exact translated label resolved for the test, and keep this case focused on the scoping invariant (textTransform is not capitalize).
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
Thanks for the follow-up here — I found one blocker in the new ProjectView title-casing spec that is keeping the web workspace tests red on this head.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.| vi.mock('../../src/i18n', () => ({ | ||
| useT: () => (key: string) => key, | ||
| })); |
There was a problem hiding this comment.
apps/web/tests/components/ProjectView.title-casing.test.tsx:23-25 only mocks useT, but this new regression spec now mounts ProjectView, and ProjectView calls useI18n() at apps/web/src/components/ProjectView.tsx:467. On the current head that makes the required web workspace suite fail with No "useI18n" export is defined on the "../../src/i18n" mock, so this PR is not merge-ready even before we get to the casing assertion itself. Please extend this mock to provide useI18n (or partially import the real i18n module and override only the pieces this test needs) so the new regression test covers the title-casing rule without breaking pnpm --filter @open-design/web test.
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
@neogenix I re-checked the current head and the changed ranges now keep the capitalization fix scoped to project-only surfaces: the workspace tab strip and overflow list use project-only modifier classes, the Designs tab covers both grid and kanban project cards, and the new regression specs exercise those paths without reintroducing the earlier selector bleed. Thanks for iterating through the follow-up fixes 🙌
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.
|
Fixed the |
|
@neogenix thanks for putting this together and for covering the surfaces carefully. After looking through the full diff, I don't think we should take this PR's direction. Project names are user-authored content, so the UI should preserve the casing that was entered or stored instead of applying display-layer title casing. CSS I also checked the full PR and it is essentially all in support of this display-layer capitalization behavior: CSS rules, class plumbing, and tests that assert If generated default project names look too lowercase, the better fix is to improve the name generation/source value and then display that value as-is. For visual fixtures, we can also seed title-cased fixture names directly. I'm going to close this PR for now, but I appreciate the effort here. |
Thanks for the comprehensive review, and great communication ❤️ |
3 participants
Why
Project names rendered all-lowercase across the design-files page header
and every other site that displays them, including the workspace tab
bar, the workspace tab overflow popover, the designs-tab cards, and the
recent-projects strip. The stored value is whatever the user typed at
project-create time; the display layer was just not styling it.
The original scope of this fix targeted only the design-files page
header. During the 10-pass adversarial review, a reviewer identified
that capitalizing only one of five sites produces worse visual
consistency than not capitalizing any: a user with project
acme studiowould see
Acme Studioon the design-files page andacme studioeverywhere else. The fix was extended to cover all five sites.
What users will see
Project names render with capitalized first letters in:
.app-project-title .title).workspace-tab__label).workspace-tabs-list__title).design-card-name).recent-projects__card-name)This is a CSS
text-transform: capitalizerule, so the underlyingstored value is unchanged — a project named
acme studiois stillstored as
acme studioand exported asacme studio. Only therendered glyphs change.
Surface area
No i18n keys added (CSS-only). No CLI surface (pure display layer).
No data-shape change (storage untouched).
Screenshots
The fix is a one-glyph-per-word change visible only when a project name
has lowercase characters. Reproduction: create a project named
acme studio; onmain, every display site showsacme studio; onthis branch, every display site shows
Acme Studiowhile the storedname remains
acme studio.Bug fix verification
apps/web/tests/components/ProjectView.title-casing.test.tsx—mounts
ProjectViewwith a lowercase name, injects the relevantrules from
index.cssinto the test JSDOM, and assertsgetComputedStyle(titleEl).textTransform === 'capitalize'.apps/web/tests/components/css-project-name-casing.test.tsx—same assertion technique applied to each of the other four
selectors (full-component mount for
WorkspaceTabsBar,DesignsTab,RecentProjectsStrip; fixture DOM for the taboverflow list, since its visibility depends on viewport width).
main(rules absent) and green on thisbranch (rules present).
e2e/ui/project-name-casing.test.ts(6 tests):creates a real project named
'acme studio'viaPOST /api/projects,navigates to each display site, and asserts via real-browser
getComputedStylethattext-transformresolves tocapitalize.Also asserts the stored
textContentremains lowercase (proving thefix is display-only). Tests 2-5 red on
main; all 6 green here.Validation
pnpm guard(clean)pnpm typecheck(clean)pnpm --filter @open-design/web test ProjectView.title-casing— 1/1 pass
pnpm --filter @open-design/web test css-project-name-casing— 4/4 pass
pnpm exec playwright test -c playwright.config.ts project-name-casing— 6/6 pass (browser-verified in Chromium, 11.0s)
Trade-off documented
text-transform: capitalizeonly touches the first character of eachwhitespace-separated word; it does not lowercase subsequent characters.
That means a brand-name project like
iPhone Studiodisplays asIPhone Studiorather thaniPhone Studio. This is acceptable for theprimary case I reported (auto-generated lowercase project names) and is
documented inline in
index.cssnear the rule. The storage-layeralternative — preserving user-typed casing exactly on create/rename —
would be more correct for brand names but is a much larger change with
its own data-migration concerns; flagged as a follow-up in the inline
comment.
Adjacent issues (out of scope)
for brand names; intentionally deferred per the trade-off note).
apps/web/tests/styles/...wasremoved during the 10-pass cleanup in favor of the behavior-level
specs above. That was technical-debt cleanup folded into the same
PR scope.
Coordination note
There is an open upstream PR #2258 ("Fix (web): long project name
rendering") that also touches
apps/web/src/components/ProjectView.tsxand
apps/web/src/index.cssin adjacent blocks (it adds atitle={project.name}tooltip attribute and small padding/margintweaks). The diffs do not overlap on the same lines and merge
mechanically. The two PRs are functionally complementary — that one
makes long names readable on hover, this one makes lowercase names
readable on first glance.